Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(ses): auto setting for DMARC #28507

Closed
wants to merge 3 commits into from
Closed

feat(ses): auto setting for DMARC #28507

wants to merge 3 commits into from

Conversation

watany-dev
Copy link
Contributor

Introducing DMARC functionality to Amazon SES Construct to enhance email security by authenticating sender identity. This feature allows users to enable DMARC for their email domains with minimal configuration.

  • Changes
    Added autoDmarc flag in EmailIdentity for automatic DMARC TXT record creation.
    Optional dmarcReportEmail for custom DMARC report destination.

  • Example

new ses.EmailIdentity(this, 'Identity', {
  identity: ses.Identity.publicHostedZone(myHostedZone),
  autoDmarc: true,
  dmarcReportEmail: 'custom-report@domain.com',
});

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@github-actions github-actions bot added the p2 label Dec 28, 2023
@aws-cdk-automation aws-cdk-automation requested a review from a team December 28, 2023 08:43
@github-actions github-actions bot added the distinguished-contributor [Pilot] contributed 50+ PRs to the CDK label Dec 28, 2023
@watany-dev
Copy link
Contributor Author

The integ test should have passed locally.

kaizencc
kaizencc previously requested changes Dec 29, 2023
View reviewed changes
Copy link
Contributor

@kaizencc kaizencc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just one comment @watany-dev, which should also have its own unit test, and then we are good to go. Thanks!

packages/aws-cdk-lib/aws-ses/lib/email-identity.ts Outdated
@@ -419,6 +434,16 @@ export class EmailIdentity extends Resource implements IEmailIdentity {
});
}

if (props.autoDmarc && props.identity.hostedZone) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If props.dmarcReportEmail is set but props.autoDmarc is not, then the email will be ignored, which I argue is unexpected. Instead, we should assume that autoDmarc is true if dmarcReportEmail is set.

@mergify mergify bot dismissed kaizencc’s stale review December 29, 2023 13:27

Pull request has been modified.

@github-actions github-actions bot mentioned this pull request Jan 1, 2024
Closed
@kaizencc kaizencc changed the title feat(ses): Auto Setting for DMARC feat(ses): auto setting for DMARC Jan 2, 2024
kaizencc
kaizencc requested changes Jan 2, 2024
View reviewed changes
Copy link
Contributor

@kaizencc kaizencc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good @watany-dev but looks like the integ test needs updating?

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 6110517
  • Result: FAILED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation
Copy link
Collaborator

This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

@aws-cdk-automation
Copy link
Collaborator

This PR has been deemed to be abandoned, and will be automatically closed. Please create a new PR for these changes if you think this decision has been made in error.

@aws-cdk-automation aws-cdk-automation added the closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. label Feb 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kaizencc kaizencc kaizencc requested changes

Assignees
No one assigned
Labels
closed-for-staleness This issue was automatically closed because it hadn't received any attention in a while. distinguished-contributor [Pilot] contributed 50+ PRs to the CDK p2
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@watany-dev @aws-cdk-automation @kaizencc